Skip to content

Setup workflows#43

Open
siva-spotdraft wants to merge 5 commits intomasterfrom
setup-workflows
Open

Setup workflows#43
siva-spotdraft wants to merge 5 commits intomasterfrom
setup-workflows

Conversation

@siva-spotdraft
Copy link
Copy Markdown

@siva-spotdraft siva-spotdraft commented Sep 30, 2025

User description

Summary by CodeRabbit

  • Chores
    • Renamed the published package to @spotdraft/liquidjs—update installation/imports accordingly.
    • Added a master workflow to build and publish on master pushes using Node.js 20 and Google Artifact Registry; includes auth, dependency install, and publish steps.
    • Added a PR workflow to run builds and a dry-run publish (using Node.js 24) for pull request validation.

Generated description

Below is a concise technical summary of the changes proposed in this PR:
Configures GitHub Actions workflows for automated building and publishing to Google Artifact Registry. Updates the package name to @spotdraft/liquidjs to align with organizational scoping requirements.

TopicDetails
CI/CD Workflows Implements master.yaml and pr.yaml workflows to handle authentication with Google Cloud, dependency installation, and package publishing.
Modified files (2)
  • .github/workflows/master.yaml
  • .github/workflows/pr.yaml
Latest Contributors(0)
UserCommitDate
Package Config Renames the project to @spotdraft/liquidjs within package.json to support scoped registry publishing.
Modified files (1)
  • package.json
Latest Contributors(2)
UserCommitDate
sumanth-spotdraftAllow-date-and-term-ca...March 31, 2020
harttle@harttle.com3.1.0January 12, 2018
This pull request is reviewed by Baz. Review like a pro on (Baz).

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Sep 30, 2025

Walkthrough

Adds two GitHub Actions workflows: one that publishes on pushes to master and one that runs a PR dry-run; and renames the published package from liquidjs to @spotdraft/liquidjs. No source code or API changes.

Changes

Cohort / File(s) Summary
CI workflow: publish on master
.github/workflows/master.yaml
New workflow triggered on pushes to master: checks out code; sets up Node.js v20; authenticates to GCP via Workload Identity and a service account; configures gcloud project; obtains Artifact Registry npm token; runs npm install and npm publish.
CI workflow: PR dry-run
.github/workflows/pr.yaml
New PR workflow triggered on PR events against master: checks out code; sets Node.js v24; sets permissions (contents read, id-token write, packages write); authenticates to GCP via Workload Identity; configures gcloud and npm registry token; runs npm install and a dry-run npm publish.
Package metadata rename
package.json
Updated package name from liquidjs to @spotdraft/liquidjs. No other metadata, version, scripts, dependencies, or public API changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Dev as Developer
    participant GH as GitHub Actions
    participant GCP as Google Cloud (Workload Identity)
    participant AR as Artifact Registry (npm)

    Dev->>GH: Push to master
    activate GH
    GH->>GH: actions/checkout\nsetup-node@v20
    GH->>GCP: Authenticate (workload identity)\nAssume service account
    GCP-->>GH: Access token
    GH->>AR: Configure npm auth (token)
    GH->>GH: npm install
    GH->>AR: npm publish
    AR-->>GH: Publish result
    deactivate GH
Loading
sequenceDiagram
    autonumber
    participant Dev as Developer
    participant GH as GitHub Actions (PR workflow)
    participant GCP as Google Cloud (Workload Identity)
    participant AR as Artifact Registry (npm)

    Dev->>GH: Open/sync/reopen PR -> target master
    activate GH
    GH->>GH: actions/checkout\nsetup-node@v24
    GH->>GCP: Authenticate (workload identity)\nAssume service account
    GCP-->>GH: Access token
    GH->>AR: Configure npm auth (token)
    GH->>GH: npm install
    GH->>AR: npm publish (dry-run)
    AR-->>GH: Dry-run result
    deactivate GH
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

I thump my paws—release day cheer!
A master push, the clouds draw near.
Tokens twirl, the registry sings,
Our name now scoped with brighter rings.
Hop hop—package launched with glee! 🐇🚀

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Setup workflows” succinctly captures the primary purpose of this changeset, which is to add and configure GitHub Actions workflows, making it clear and directly related to the main modifications in the pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch setup-workflows

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
package.json (1)

2-2: Add publishConfig to lock the publish target.

Right now a developer running npm publish locally would fall back to the default registry if their .npmrc isn’t primed, risking an accidental push to npmjs. Add a publishConfig.registry pointing at Artifact Registry so every publish path stays scoped correctly.(docs.github.com)

   "name": "@spotdraft/liquidjs",
   "version": "3.1.0",
   ...
   "postversion": "git push && git push --tags"
-  }
+  },
+  "publishConfig": {
+    "registry": "https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/"
+  }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ce1515 and f03ecfb.

📒 Files selected for processing (2)
  • .github/workflows/master.yaml (1 hunks)
  • package.json (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/master.yaml

8-8: label "ubicloud-standard-2" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

Comment on lines +35 to +40
- name: Configure NPM to use Artifact Registry
run: |
TOKEN=$(gcloud auth print-access-token)
rm -rf .npmrc
echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add always-auth=true so npm actually sends the token.

Artifact Registry’s own guidance keeps an always-auth=true entry alongside the _authToken. Without it, npm can skip attaching the token on preliminary GET/HEAD requests when publishing, which leads to intermittent 401s during npm publish. Please add the flag when building .npmrc.(cloud.google.com)

           TOKEN=$(gcloud auth print-access-token)
           rm -rf .npmrc
-          echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
+          echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:always-auth=true" >> .npmrc
+          echo "//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
           echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Configure NPM to use Artifact Registry
run: |
TOKEN=$(gcloud auth print-access-token)
rm -rf .npmrc
echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc
- name: Configure NPM to use Artifact Registry
run: |
TOKEN=$(gcloud auth print-access-token)
rm -rf .npmrc
echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:always-auth=true" >> .npmrc
echo "//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc
🤖 Prompt for AI Agents
In .github/workflows/master.yaml around lines 35 to 40, the workflow writes an
.npmrc with the _authToken and registry but omits the always-auth flag; update
the script that builds .npmrc to append an always-auth=true entry (for the
registry in question) immediately after writing the _authToken so npm will
always send the token and avoid intermittent 401s during publish.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f03ecfb and ec25196.

📒 Files selected for processing (1)
  • .github/workflows/pr.yaml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pr.yaml

8-8: label "ubicloud-standard-2" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

Comment on lines +37 to +40
TOKEN=$(gcloud auth print-access-token)
rm -rf .npmrc
echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix _authToken quoting to avoid authentication failure.

.npmrc keeps the double quotes you echo around ${TOKEN}, so npm sends Bearer "token" and Artifact Registry rejects the publish (dry-run or real). Drop the quotes and write the file in one shot.

-          TOKEN=$(gcloud auth print-access-token)
-          rm -rf .npmrc
-          echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
-          echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc
+          TOKEN="$(gcloud auth print-access-token)"
+          cat <<'EOF' > .npmrc
+//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=${TOKEN}
+@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/
+EOF

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In .github/workflows/pr.yaml around lines 37 to 40, the current echo adds
literal double quotes around the auth token and appends each line separately;
change it to write the .npmrc in a single write operation and remove the
surrounding quotes so the _authToken is written as ...:_authToken=<TOKEN> (no
quotes). Ensure the command writes both registry lines to .npmrc atomically
(overwrite, not append) and that the token is inserted raw so npm sends Bearer
<token> without embedded quotes.

Comment on lines 1 to 4
{
"name": "liquidjs",
"name": "@spotdraft/liquidjs",
"version": "3.1.0",
"description": "Liquid template engine by pure JavaScript: compatible to shopify, easy to extend.",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renaming the package to @spotdraft/liquidjs while the publish workflow only authenticates/publishes to the scoped registry means npm install liquidjs stops receiving updates yet README/install docs still point to the unscoped name; can we keep publishing an alias named liquidjs or explicitly document the new scoped package before the contract is broken?

Suggested change
{
"name": "liquidjs",
"name": "@spotdraft/liquidjs",
"version": "3.1.0",
"description": "Liquid template engine by pure JavaScript: compatible to shopify, easy to extend.",
{
"name": "liquidjs",
"version": "3.1.0",
"description": "Liquid template engine by pure JavaScript: compatible to shopify, easy to extend.",

Finding type: Breaking Changes

Comment on lines +17 to +20
- name: Setup Node.js
uses: actions/setup-node@v5
with:
node-version: '24'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR workflow now sets node-version: '24' while the master publish workflow still uses Node 20, so the dry-run on PRs no longer validates the actual publish environment and Node 20–specific build/publish regressions can slip through; can we pin the same Node version in both workflows?

Suggested change
- name: Setup Node.js
uses: actions/setup-node@v5
with:
node-version: '24'
- name: Setup Node.js
uses: actions/setup-node@v5
with:
node-version: '20'

Finding type: Logical Bugs

Comment on lines +35 to +39
- name: Configure NPM to use Artifact Registry
run: |
TOKEN=$(gcloud auth print-access-token)
rm -rf .npmrc
echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

echo -e "..._authToken="$TOKEN"" expands the freshly minted GCP access token inside the logged shell command, so the raw token is emitted in every workflow log (same lines exist in pr.yaml); can we mask the token (e.g. echo "::add-mask::$TOKEN") or otherwise avoid printing the value when writing .npmrc?

Suggested change
- name: Configure NPM to use Artifact Registry
run: |
TOKEN=$(gcloud auth print-access-token)
rm -rf .npmrc
echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
- name: Configure NPM to use Artifact Registry
run: |
TOKEN=$(gcloud auth print-access-token)
echo "::add-mask::$TOKEN" && rm -rf .npmrc && echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc

Finding type: Basic Security Patterns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant